Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARM64] PowerRename #18002

Merged
merged 8 commits into from
May 5, 2022
Merged

Conversation

snickler
Copy link
Collaborator

@snickler snickler commented May 2, 2022

Summary of the Pull Request

What is this about:
Adding configuration to PowerRename to ensure it builds correctly for ARM64

What is included in the PR:
image

How does someone test / validate:
Set configuration to Release|ARM64, set PowerRenameUI as the startup project, build & run.

Quality Checklist

  • Linked issue: Support ARM platform #490
  • Communication: I've discussed this with core contributors in the issue.
  • Tests: Added/updated and all pass
  • Installer: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Docs: Added/ updated
  • Binaries: Any new files are added to WXS / YML

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@github-actions
Copy link

github-actions bot commented May 2, 2022

@check-spelling-bot Report

🔴 Please review

See the files view or the action log for details.

Unrecognized words (6)

debugbreak
IResource
iwindow
restrictederrorinfo
stefa
targetentrypoint

Previously acknowledged words that are now absent BGSOUNDS CLIENTPULL dispid DISPIDAMBIENTDLCONTROL DLACTIVEXCTLS DLCONTROL DLIMAGES DOWNLOADONLY FANCYZONESWINDOWSTYLES FORCEOFFLINE FRAMEDOWNLOAD gsuberland HFONT Htmdid ICore IDCANCEL IDOK INITDIALOG IReflect IWindows IXaml lamotile METACHARSET mirophone mshtmdid NETFX netstandard Nvidia otating Postion preperty Redist ruleset RUNACTIVEXCTLS serizalization settingsv Setttings sourceid testtrocess Toolchain VDId xbf XBind XInstance
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:snickler/PowerToys.git repository
on the snickler/pr_winui3 branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/PowerToys/issues/comments/1115420725" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u
If the flagged items do not appear to be text

If items relate to a ...

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

  • binary file.

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@snickler snickler closed this May 2, 2022
@snickler snickler reopened this May 2, 2022
@github-actions

This comment was marked as outdated.

@snickler snickler changed the title Removed Project Config and update PlatformToolset to v143 [ARM64] PowerRename May 2, 2022
@crutkas
Copy link
Member

crutkas commented May 3, 2022

🔥🔥🔥

@microsoft microsoft deleted a comment from github-actions bot May 4, 2022
PowerToys.sln Outdated
@@ -520,8 +520,8 @@ Global
{89F34AF7-1C34-4A72-AA6E-534BCF972BD9}.Release|x64.ActiveCfg = Release|x64
{89F34AF7-1C34-4A72-AA6E-534BCF972BD9}.Release|x64.Build.0 = Release|x64
{89F34AF7-1C34-4A72-AA6E-534BCF972BD9}.Release|x86.ActiveCfg = Release|x64
{2BE46397-4DFA-414C-9BD4-41E4BBF8CB34}.Debug|ARM64.ActiveCfg = Debug|ARM64
{2BE46397-4DFA-414C-9BD4-41E4BBF8CB34}.Debug|ARM64.Build.0 = Debug|ARM64
{2BE46397-4DFA-414C-9BD4-41E4BBF8CB34}.Debug|ARM64.ActiveCfg = Debug|x64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting with this and all changes bellow are ARM64->X64? Is that ok?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad you caught this. Those projects shouldn't have been set back to x64. I have a feeling it happened by VS, since I changed it in the UI rather than replacing the string manually

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefansjfw - I've made commits to resolve that problem.

Copy link
Collaborator

@stefansjfw stefansjfw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🚀

@stefansjfw stefansjfw merged commit 1770fe7 into microsoft:stefan/pr_winui3 May 5, 2022
stefansjfw added a commit that referenced this pull request May 9, 2022
* Migrate PowerRename to Unpackaged WinUI3

* Removed Project Config and update PlatformToolset to v143

* Updated solution config

* Migrate PowerRename to Unpackaged WinUI3

* Fixed configs changed from ARM64 build

* Left one project out of fix

Co-authored-by: Stefan Markovic <stefan@janeasystems.com>
Co-authored-by: Stefan Markovic <57057282+stefansjfw@users.noreply.github.com>
stefansjfw added a commit that referenced this pull request May 11, 2022
…8171)

* Migrate PowerRename to Unpackaged WinUI3

* [ARM64] PowerRename (#18002)

* Migrate PowerRename to Unpackaged WinUI3

* Removed Project Config and update PlatformToolset to v143

* Updated solution config

* Migrate PowerRename to Unpackaged WinUI3

* Fixed configs changed from ARM64 build

* Left one project out of fix

Co-authored-by: Stefan Markovic <stefan@janeasystems.com>
Co-authored-by: Stefan Markovic <57057282+stefansjfw@users.noreply.github.com>

* Minor fixes

* Remove PowerRenameUILib from signing list - doesn't exist anymore

* Remove PowerRenameUILib from move_uwp_resources.ps1

* Bring back old dir name to see if localization is preserved
Remove move_uwp_resources.ps1 - not needed anymore

* Remove UWP localization docs part

* Fix minor UI quirk

Co-authored-by: Jeremy Sinclair <4016293+snickler@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants